fetch ids without instantiating records using pluck #584
Conversation
|
Nice! 👍 we don't need the test, the gem support 3.2+ Can you add also a note in the change log under 3.4.0 ? |
ashanbrown
force-pushed
the
fix_mysql_record_instantiation
branch
2 times, most recently
from
bfd4123 to
518ed6f
Compare
ashanbrown
changed the title
|
I've amended the commit to always use pluck (for mysql only ... not sure about the other path). I've also updated the changelog and fixed the markdown. The gemspec requires active record >= 3. Maybe it should be updated to |
| @@ -12,13 +12,15 @@ As such, a _Feature_ would map to either major or minor. A _bug fix_ to a patch. | |||
| * Performance | |||
| * Misc | |||
|
|
|||
| ### [3.4.0 / 2014-08-29](Master [changes](https://github.com/mbleigh/acts-as-taggable-on/compare/v3.3.0...v3.4.0) | |||
| ### [3.4.0 / 2014-08-29](https://github.com/mbleigh/acts-as-taggable-on/compare/v3.3.0...v3.4.0) | |||
|
|
|||
| * Features | |||
| * [@ProGM Support for custom parsers for tags](https://github.com/mbleigh/acts-as-taggable-on/pull/579) | |||
| * [@damzcodes #577 Popular feature](https://github.com/mbleigh/acts-as-taggable-on/pull/577) | |||
| * Fixes | |||
| * [@twalpole Update for rails edge (4.2)] | |||
There was a problem hiding this comment.
* [@twalpole Update for rails edge (4.2)](https://github.com/mbleigh/acts-as-taggable-on/pull/583)
There was a problem hiding this comment.
Can you correct this one too ?
|
I already fixed the gemspec |
ashanbrown
force-pushed
the
fix_mysql_record_instantiation
branch
from
518ed6f to
9616b3f
Compare
|
I've updated the request to just replace select with pluck. As I read the code, mysql doesn't enjoy the benefit of making a single query, which is unfortunate. I know rails can automatically generated nested queries, but I'm having trouble getting that to work and I don't know what version that started with. |
|
Look good to me. Thank you. We can optimize/fix later. |
ashanbrown
force-pushed
the
fix_mysql_record_instantiation
branch
2 times, most recently
from
6c4cb3b to
af95d94
Compare
|
Got it. Put the link in the changelog. Regarding the the separate queries, it looks like that's why the code was refactored a bit (#457) and it fixes a problem with pagination. I wonder if we could just check if the query has a |
|
Core need a full rewrite to use the rails way. |
fetch ids without instantiating records using pluck
…ntiation fetch ids without instantiating records using pluck
Not only is pluck more efficient for fetching ids, but instantiating a record with only an id can cause problems isfthere are after_find hooks that expect to have all the columns populated. However, pluck was a rails 3.2 addition, so we have to test for it since this gem supports back to rails 3.0.0.